Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust headline size based on screen size #849

Merged

Conversation

ductbuix
Copy link
Contributor

@ductbuix ductbuix commented Nov 22, 2023

Fix #757 by adjusting headline font size for mobile view and its responsive container.

πŸ€–[deprecated] Generated by Copilot at 6384d6a

Summary

πŸ“±πŸ› οΈπŸŽ¨

Enhanced the design and functionality of the Hero component on the home page. Made it adapt to different screen sizes and resolutions using src/redesign/components/HomeLanding.jsx.

The Hero component was too plain
It needed some tweaks to look sane
On different devices
It had some vices
But now it adapts to the screen

Walkthrough

  • Adjust the height and width of the Hero and HeroText components to be responsive to the screen size (link, link)
  • Reduce the font size of the HeroTitle component on mobile to avoid text wrapping or clipping (link)

@ductbuix ductbuix changed the title fix: Adjust headline size based on screen size Adjust headline size based on screen size Nov 22, 2023
@nikhilwoodruff
Copy link
Contributor

Thanks for this @ductbuix- just testing it out locally I get this though- any ideas?
image

nikhilwoodruff

This comment was marked as duplicate.

Copy link
Contributor

@nikhilwoodruff nikhilwoodruff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

@ductbuix
Copy link
Contributor Author

ductbuix commented Nov 27, 2023

Thanks for this @ductbuix- just testing it out locally I get this though- any ideas? image

@nikhilwoodruff Can you give me more information (screen size, or any condition)? I've tried several responsive sizes but it works still well for the mobile screens.

@anth-volk
Copy link
Collaborator

Could this at all be a Safari thing? I know you test in Safari, @nikhilwoodruff, and I've found in the past that I have to use more workarounds, vendor prefixes, etc., when working with it.

@anth-volk
Copy link
Collaborator

Interestingly, post-changes, I don't get what Nikhil had mentioned, but I do get this at around 960px:

Screen Shot 2023-11-27 at 1 07 34 PM

@ductbuix
Copy link
Contributor Author

Could this at all be a Safari thing? I know you test in Safari, @nikhilwoodruff, and I've found in the past that I have to use more workarounds, vendor prefixes, etc., when working with it.

Oh, yes! I see, the Safari, let me take a look on it.

Interestingly, post-changes, I don't get what Nikhil had mentioned, but I do get this at around 960px:

Yea, I initially focused on the issue title so I've only fixed for mobile.

So I will try to figure out what happened with Safari and fix the responsive for all screen. Btw, tell me if you know anything about why it broke in the Safari. @nikhilwoodruff @anth-volk

@nikhilwoodruff
Copy link
Contributor

Thanks so much @ductbuix. Not sure I have any useful details to add on the root cause but let me know if you can't reproduce it.

@ductbuix
Copy link
Contributor Author

Thanks so much @ductbuix. Not sure I have any useful details to add on the root cause but let me know if you can't reproduce it.

Yea, I found the issue, but the fix might need some more changes. I'm on it.

@ductbuix
Copy link
Contributor Author

Hi @anth-volk @nikhilwoodruff , I've fixed the responsive header, please check it out.

@nikhilwoodruff
Copy link
Contributor

Looks great on my machine, thanks @ductbuix! @anth-volk I think the merge conflict is from your PR, would you mind taking a look at it?

Copy link
Contributor

@nikhilwoodruff nikhilwoodruff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to merge upon the merge conflict being resolved

@ductbuix
Copy link
Contributor Author

Good to merge upon the merge conflict being resolved

@nikhilwoodruff It looks good now.

@MaxGhenis MaxGhenis merged commit f550a26 into PolicyEngine:master Nov 29, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Homepage headline doesn't fit in box on mobile
4 participants